Skip to content

fix(transport): close inbound link when accept fails or times out - #2736

Closed
eeshsaxena wants to merge 1 commit into
eclipse-zenoh:mainfrom
eeshsaxena:fix-accept-link-socket-leak
Closed

fix(transport): close inbound link when accept fails or times out#2736
eeshsaxena wants to merge 1 commit into
eclipse-zenoh:mainfrom
eeshsaxena:fix-accept-link-socket-leak

Conversation

@eeshsaxena

@eeshsaxena eeshsaxena commented Aug 8, 2026

Copy link
Copy Markdown

Fixes #2734.

The bug

The acceptor task wraps accept_link in tokio::time::timeout and only looks at the outer Result with .is_err():

if tokio::time::timeout(
    c_manager.config.unicast.accept_timeout,
    super::establishment::accept::accept_link(link, &c_manager),
)
.await
.is_err()
{
    tracing::debug!("Failed to accept link before deadline ...");
}

timeout(...).await returns Result<ZResult<()>, Elapsed>. .is_err() is only true for Elapsed, so the Ok(Err(e)) case (an early handshake error, for example a malformed InitSyn) is silently dropped. More importantly, neither the error case nor the timeout case closes the link.

accept_link itself only closes the link on its FSM reject path (the step! macro, which sends a Close with a reason). Errors that return earlier, such as a failed InitSyn decode or a ?-propagated setup error, and the timeout, never reach that path, so the socket is left open. It then lingers in CLOSE-WAIT until the process restarts. A peer that begins a handshake and then goes away leaks one descriptor each time, which is the repro in the issue.

The fix

Match on the full Result and close the link on both failure paths, mirroring what the accept_pending (DoS) branch a few lines above already does with link.close():

  • Ok(Ok(())): success, the established transport keeps its own handle, so just drop this one.
  • Ok(Err(e)): log the error and close the link.
  • Err(_) (timeout): log the deadline miss and close the link.

accept_link takes the link by value, so I keep a cheap clone (LinkUnicast is Arc-backed) to close afterwards. The close only runs after timeout(...).await has returned, i.e. after accept_link's future has completed or been dropped, so there is no concurrent access to the link. If accept_link already closed the link on its step! path, the extra close() is a harmless no-op whose error is ignored.

Testing

I don't have a Rust toolchain on this machine to run the suite, and there is no in-tree mock LinkUnicastTrait to unit test the acceptor task against, so I verified this by reading. The reproduction and CLOSE-WAIT count in #2734 are the behavioral check: with this change the link is closed on the failure paths instead of leaking.

Note: I still need to get the Eclipse ECA sorted out for my email, will make sure the DCO/ECA check is green.


🏷️ Label-Based Checklist

No specific label requirements detected.

Current labels: No labels

Add one of these labels to this PR to see relevant checklist items: api-sync, breaking-change, bug, ci, dependencies, documentation, enhancement, new feature, internal

This section updates automatically when labels change.

The acceptor task ran accept_link inside tokio::time::timeout and only
checked the outer Result with .is_err(). That is true only when the
timeout elapses, so the Ok(Err(e)) case, an early handshake error such as
a malformed InitSyn, was discarded. Neither the error case nor the timeout
case closed the link.

accept_link only closes the link on its FSM reject path (the step! macro).
Errors that return earlier, and the timeout, leave the socket open. It then
sits in CLOSE-WAIT until the process restarts, so a peer that starts a
handshake and walks away leaks a file descriptor each time.

Match on the full Result and close the link on both failure paths, the same
way the accept_pending (DoS) branch just above already does. On success the
established transport keeps its own handle, and the close only runs after the
timeout returns, so accept_link's future is already gone and there is no
concurrent access to the link.

Signed-off-by: eeshsaxena <eeshsaxena@gmail.com>
@eeshsaxena

Copy link
Copy Markdown
Author

Closing this to clear out my older open PRs. Nothing wrong with the change on my side, I am just tidying up a backlog. If it is still useful to you, say so and I will reopen it and rebase it on current main.

@eeshsaxena eeshsaxena closed this Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inbound link is never closed when accept_link() returns Err — sockets leak in CLOSE-WAIT

1 participant